Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add e2e tests for proxy option #3748

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

snitin315
Copy link
Member

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

Add e2e tests for proxy option.

Breaking Changes

None

Additional Info

No

}
},
},
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should refactor out tests:

  1. proxyOptionsObject
  2. proxyOptionsArray
  3. proxyOptionsObjectWithMiltipleContext
  4. ...
  5. proxyOptionOfArrayWithoutTarget
  6. proxyWithPath
  7. ...
  8. proxyWithByPass
    9 ...

Now we have mixed tests it is bad for refactor and bad for testing, the latest regression will be due this, so let's refactor our tests


exports[`proxy option should handles external websocket upgrade with webSocketServerType: ws Should receive response: response status 1`] = `426`;

exports[`proxy option should handles external websocket upgrade with webSocketServerType: ws Should receive response: response text 1`] = `"Upgrade Required"`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, need to fix it

@snitin315 snitin315 marked this pull request as draft August 29, 2021 12:45
@alexander-akait
Copy link
Member

@snitin315 let's finish tests, thanks for help

@snitin315
Copy link
Member Author

All other tests are moved to E2E, let's finish this one too.

@harish-sethuraman
Copy link
Contributor

@alexander-akait Would like to know what is pending in this PR. I'd like to close it to complete :)

@snitin315
Copy link
Member Author

I think need to fix CI

@harish-sethuraman
Copy link
Contributor

Can you re trigger
I dont see any logs since it has been a long time

@snitin315 snitin315 closed this Jul 23, 2022
@snitin315 snitin315 reopened this Jul 23, 2022
@snitin315
Copy link
Member Author

I see snapshots needs to be updated. I will push a commit here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants